Skip to content

Conversation

tbaederr
Copy link
Contributor

We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.

We need to evaluate both the True/False expressions of a conditional
operator as well as the LHS/RHS of a binary operator in more cases.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Apr 30, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 30, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.


Full diff: https://github.com/llvm/llvm-project/pull/137962.diff

5 Files Affected:

  • (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.h (+1)
  • (modified) clang/lib/AST/ByteCode/Compiler.cpp (+17-4)
  • (modified) clang/lib/AST/ByteCode/EvalEmitter.h (+3)
  • (modified) clang/lib/AST/ByteCode/Interp.cpp (+1-1)
  • (modified) clang/test/Sema/integer-overflow.c (+1)
diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.h b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
index 5c7482923386e..9e9dd5e87cd7a 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.h
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
@@ -60,6 +60,7 @@ class ByteCodeEmitter {
 
   /// We're always emitting bytecode.
   bool isActive() const { return true; }
+  bool checkingForUndefinedBehavior() const { return false; }
 
   /// Callback for local registration.
   Local createLocal(Descriptor *D);
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 9a1e61b54b8be..b4ac832946233 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -866,12 +866,14 @@ bool Compiler<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
 
   // Assignments require us to evalute the RHS first.
   if (BO->getOpcode() == BO_Assign) {
-    // We don't support assignments in C.
-    if (!Ctx.getLangOpts().CPlusPlus)
-      return this->emitInvalid(BO);
 
     if (!visit(RHS) || !visit(LHS))
       return false;
+
+    // We don't support assignments in C.
+    if (!Ctx.getLangOpts().CPlusPlus && !this->emitInvalid(BO))
+      return false;
+
     if (!this->emitFlip(*LT, *RT, BO))
       return false;
   } else {
@@ -2364,8 +2366,19 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
       return false;
   }
 
-  if (!this->visitBool(Condition))
+  if (!this->visitBool(Condition)) {
+    // If the condition failed and we're checking for undefined behavior
+    // (which only happens with EvalEmitter) check the TrueExpr and FalseExpr
+    // as well.
+    if (this->checkingForUndefinedBehavior()) {
+      if (!this->discard(TrueExpr))
+        return false;
+      if (!this->discard(FalseExpr))
+        return false;
+    }
     return false;
+  }
+
   if (!this->jumpFalse(LabelFalse))
     return false;
   if (!visitChildExpr(TrueExpr))
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.h b/clang/lib/AST/ByteCode/EvalEmitter.h
index f53f86c31ec1e..18adf8656c0d5 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.h
+++ b/clang/lib/AST/ByteCode/EvalEmitter.h
@@ -69,6 +69,9 @@ class EvalEmitter : public SourceMapper {
   /// Since expressions can only jump forward, predicated execution is
   /// used to deal with if-else statements.
   bool isActive() const { return CurrentLabel == ActiveLabel; }
+  bool checkingForUndefinedBehavior() const {
+    return S.checkingForUndefinedBehavior();
+  }
 
   /// Callback for registering a local.
   Local createLocal(Descriptor *D);
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 4d89f23401db5..40a6965b426ed 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1336,7 +1336,7 @@ static bool getField(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
     return false;
   }
 
-  if (Off > Ptr.block()->getSize())
+  if ((Ptr.getByteOffset() + Off) >= Ptr.block()->getSize())
     return false;
 
   S.Stk.push<Pointer>(Ptr.atField(Off));
diff --git a/clang/test/Sema/integer-overflow.c b/clang/test/Sema/integer-overflow.c
index 3141443c73305..30a47aa5f6ad6 100644
--- a/clang/test/Sema/integer-overflow.c
+++ b/clang/test/Sema/integer-overflow.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu -fexperimental-new-constant-interpreter
 typedef unsigned long long uint64_t;
 typedef unsigned int uint32_t;
 

@tbaederr tbaederr merged commit c51be1b into llvm:main May 1, 2025
15 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented May 1, 2025

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu running on as-builder-9 while building clang at step 16 "test-check-lldb-api".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/8388

Here is the relevant piece of the build log for the reference
Step 16 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure)
******************** TEST 'lldb-api :: functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py' FAILED ********************
Script:
--
/usr/bin/python3.12 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --libcxx-include-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/c++/v1 --libcxx-include-target-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/aarch64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib/aarch64-unknown-linux-gnu --arch aarch64 --build-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/lldb --compiler /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang --dsymutil /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --lldb-obj-root /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb --lldb-libs-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --platform-url connect://jetson-agx-2198.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot /mnt/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux --skip-category=lldb-server /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/functionalities/thread/concurrent_events -p TestConcurrentManyWatchpoints.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision c51be1be3ac9c66fc0c598298edd1fd224c1da07)
  clang revision c51be1be3ac9c66fc0c598298edd1fd224c1da07
  llvm revision c51be1be3ac9c66fc0c598298edd1fd224c1da07

Watchpoint 1 hit:
old value: 0
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
old value: 1
new value: 1

Watchpoint 1 hit:
...

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
We need to evaluate both the True/False expressions of a conditional
operator as well as the LHS/RHS of a binary operator in more cases.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
We need to evaluate both the True/False expressions of a conditional
operator as well as the LHS/RHS of a binary operator in more cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants